fix: preserve case-sensitive URL parts#2017
Conversation
| assert output == expected_output | ||
|
|
||
|
|
||
| def test_compute_unique_key_preserves_case_sensitive_path_and_query() -> None: |
There was a problem hiding this comment.
Could you please write it as a parametrized test with 1 assert and 3 sets of parameters and ids explaining a specific test case (path_key, query_value_key, query_name_key)
There was a problem hiding this comment.
Thanks! Updated the regression to a parametrized test with one assert and the requested ids: path_key, query_value_key, and query_name_key.
I also rebased onto the current origin/master and re-ran:
uv run pytest tests/unit/_utils/test_requests.py -q(20 passed)uv run poe lintuv run poe type-checkgit diff origin/master --check
The Semgrep PR check is passing on the updated head 7108d4ba45093e60a1166676ec84e65a1cc654cb as well.
roian6
force-pushed
the
fix/normalize-url-case-sensitive-2008
branch
from
a3080be to
7108d4b
Compare
|
Hi @roian6, thanks for the PR. However, this issue is currently milestoned for v2 because the fix would introduce breaking changes to how requests are represented in storages. Because of that, this needs to be handled carefully as part of a major version update, along with an appropriate upgrade guide. I'm going to close this for now. |
4 participants
Description
normalize_urlwhile still relying onyarlto canonicalize scheme/host casing.Issues
normalize_urllowercases the entire URL, silently deduplicating case-distinct pages #2008Testing
uv run pytest tests/unit/_utils/test_requests.py -quv run poe lintuv run poe type-checkgit diff --checkI also attempted
uv run poe unit-tests. After installing Playwright browsers, the suite reached2152 passed, 6 skippedbut still failed intests/unit/browsers/test_stagehand_browser_plugin.pybecause the local environment lacks the Stagehand SEA binary (stagehand-linux-arm64). That failure appears unrelated to this URL normalization change.Checklist